Skip to content

Loader Cache Build Failure Fixes - #39

Open
rleeson wants to merge 1 commit into
fix/issue-30-autoloader-cache-refreshfrom
feature/issue-30-loader-build-failure-fixes
Open

Loader Cache Build Failure Fixes#39
rleeson wants to merge 1 commit into
fix/issue-30-autoloader-cache-refreshfrom
feature/issue-30-loader-build-failure-fixes

Conversation

@rleeson

@rleeson rleeson commented Aug 5, 2026

Copy link
Copy Markdown

Description of the Change

Stacked on #35 (branches from fix/issue-30-autoloader-cache-refresh); this PR targets that branch, not develop. It closes the last path by which a stale cache could still reach a server, and resolves review nits on #35.

Build command

This covers unlikely edge cases, where a cache is rebuilt in place versus an existing build and there are permissions or space issues.

generate_cache() now verifies a non-empty cache file exists after writing and throws otherwise. Spatie's file driver discards the return values of mkdir()/file_put_contents(), so an un-writable target or a full disk produced only PHP warnings: the command printed Cached N class(es) and exited 0 having written nothing. A pipeline that does not clean would then deploy the previous build's cache — issue #30 again, now with a build artifact's authority.

Existing cache files are cleared before the write, so a failed rebuild in a writable location leaves nothing behind. Where the cache directory itself is un-writable the old file cannot be removed at all; there the guarantee is the non-zero exit, which stops the pipeline before it can ship the stale cache.

Test isolation

The trait-level @runTestsInSeparateProcesses is inherited and in effect — PHPUnit merges annotations from used traits into the class's own. A comment added in #36 claimed otherwise; corrected, so the annotation is not removed as dead weight. test_disable_constant_forces_live_discovery now carries its own @runInSeparateProcess, as its define() is process-wide and cannot be undone. setAccessible() calls dropped: no effect since PHP 8.1, deprecated in 8.5.

Nits

  • i18n text domains standardised on tenup-plugin, matching the rest of the package.
  • Trimmed the get_classes() cache/live fallback comments to concise statements.
  • Upgrade Guide lists the five status badges the debug page actually renders.
  • Debugging notes that a tenup_framework_cache_load_failed listener must be registered before the failing loader runs, so full coverage needs an mu-plugin.

Refs #30#35 carries the fix for the issue; this PR hardens the build path it introduced.

How to test the Change

  1. composer lint, composer static (PHPStan level 10) and composer test — all green (65 tests / 181 assertions).
  2. Build a cache, then make the target un-writable and rebuild:
    composer generate-class-cache -- <dir>   # writes class-loader-cache/class-loader-cache-v2.php
    chmod 555 <dir>
    composer generate-class-cache -- <dir>; echo $?
    Before this PR: Cached N class(es), exit 0, no file written. After: Failed to write the class cache to "…", exit 1.
  3. With a cache already present and class-loader-cache/ made un-writable, rebuild: exit 1 with Could not remove the existing class cache at "…", rather than a green build over a stale file.
  4. In wp-admin, confirm the badges on admin.php?page=tenup-framework-loaders match the five listed in the Upgrade Guide.

Local runs on PHP 8.5: three process-isolated tests error on an antecedent/patchwork deprecation flood, unrelated to this change. Green on 8.3.

Changelog Entry

Changed - generate_cache() fails with a RuntimeException (a non-zero exit from the build command) when no cache file was written, instead of reporting success. Any cache in a writable location is cleared before the write, so a failed rebuild cannot leave a stale file behind.
Developer - Build-command tests for un-writable targets and caches that cannot be replaced; corrected test-isolation annotations and removed no-op setAccessible() calls; i18n text domains standardised on tenup-plugin; Upgrade Guide badge list and Debugging hook-timing note corrected.

Credits

Props @rleeson, @darylldoyle

Checklist:

Build command (the one path that could still resurrect #30):
- generate_cache() now explicitly verifies a non-empty cache file exists
  as Spatie will provide false positive cache builds on un-writeable locations.
- Explicitly clear existing cache files for in place rebuilds to avoid stale files.
- Tests cover both paths: an unwritable target fails with nothing written, and a
  regenerate that cannot replace an existing cache fails the build.

Test isolation (correcting 90a3956):
- Retesting showed @runTestsInSeparateProcesses was inherited, fixed annotations
  and added the trait to test_disable_constant_forces_live_discovery to avoid confusion
- Dropped the ReflectionProperty/Method::setAccessible() calls, which have had no
  effect since PHP 8.1 and are deprecated in 8.5.

Nits from review:
- Updated all i18n text domains to tenup-plugin, matching the rest of the plugin
- Trimmed the ModuleInitialization cache/live fallback documentation to concise statements
- Revise Upgrade Guide documentation to list the five current cache badges/messages
- Debugging: note that a tenup_framework_cache_load_failed listener has to be
  registered before the failing loader runs, so full coverage needs an mu-plugin.

phpcs and phpstan (level 10) clean; phpunit green on PHP 8.3 (65 tests / 181
assertions). On local PHP 8.5 the three process-isolated tests error on a
patchwork deprecation flood, unrelated to these changes.

Refs #30

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant